Skip to content

fix(kad): close progress whenever closer in range#4934

Merged
mergify[bot] merged 1 commit intolibp2p:masterfrom
maqi:close_progress_whenever_got_closer
Apr 3, 2024
Merged

fix(kad): close progress whenever closer in range#4934
mergify[bot] merged 1 commit intolibp2p:masterfrom
maqi:close_progress_whenever_got_closer

Conversation

@maqi
Copy link
Contributor

@maqi maqi commented Nov 27, 2023 •
edited
Loading

Closer shall be calculated as closer to the seen num_results peers,
not just to be being the closest as current.

This is in parallel to the PR #4932
Split them into two to make the work easier to be discussed and reviewed.

Description

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@maqi maqi force-pushed the close_progress_whenever_got_closer branch 2 times, most recently from 2b333db to 9324fa1 Compare November 27, 2023 15:13
@maqi maqi force-pushed the close_progress_whenever_got_closer branch 2 times, most recently from 525faad to 3865016 Compare November 28, 2023 10:12
@mergify
Copy link
Contributor

mergify bot commented Dec 1, 2023

This pull request has merge conflicts. Could you please resolve them @maqi? 🙏

@maqi maqi force-pushed the close_progress_whenever_got_closer branch from 3865016 to 5257090 Compare December 5, 2023 13:08
@maqi
Copy link
Contributor Author

maqi commented Dec 5, 2023

This pull request has merge conflicts. Could you please resolve them @maqi? 🙏

PR is updated, sorry for the delay

@maqi
Copy link
Contributor Author

maqi commented Dec 5, 2023

Hi, @mxinden ,

here is the follow up fix in parallel to the PR #4932

let me know if it make sense.
thx

@mergify
Copy link
Contributor

mergify bot commented Feb 19, 2024

This pull request has merge conflicts. Could you please resolve them @maqi? 🙏

@guillaumemichel guillaumemichel self-requested a review March 14, 2024 15:40
guillaumemichel
guillaumemichel requested changes Mar 14, 2024
View reviewed changes
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maqi for looking into this, it is an important issue!

@maqi maqi force-pushed the close_progress_whenever_got_closer branch from 5257090 to bd15207 Compare April 3, 2024 06:14
@maqi
Copy link
Contributor Author

maqi commented Apr 3, 2024

Hi, @guillaumemichel ,

Really appreciate your comments and suggestions.

Sorry for the late reply, have not check back this crate for a while as had been a while not receive any feedback.

Thx again and hope this PR could be merged soon.

Best Regards,

@maqi
Copy link
Contributor Author

maqi commented Apr 3, 2024

The failing clippy as at https://github.com/libp2p/rust-libp2p/actions/runs/8534098889/job/23377901550?pr=4934
seems not related to this PR ?

guillaumemichel
guillaumemichel approved these changes Apr 3, 2024
View reviewed changes
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for your contribution @maqi!

@mergify mergify bot merged commit 3d16353 into libp2p:master Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@guillaumemichel guillaumemichel guillaumemichel approved these changes

@mxinden mxinden Awaiting requested review from mxinden

Assignees

No one assigned

Labels

send-it

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@maqi @guillaumemichel